fix: disable broken clawloop run/eval with truthful redirect#49
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request disables the legacy run and eval subcommands in the CLI, redirecting users to the unified TrainConfig runner and removing associated legacy logic. It introduces an early interception mechanism in the main function to provide helpful error messages for these disabled commands, supported by new smoke tests. Feedback was provided regarding the fragility of the subcommand detection logic, which may fail if global flags with arguments are added in the future.
| # so stale muscle memory (`clawloop run --bench entropic ...`) gets the | ||
| # redirect instead of an opaque "unrecognized arguments" error. | ||
| argv = sys.argv[1:] | ||
| leading = next((a for a in argv if not a.startswith("-")), None) |
There was a problem hiding this comment.
The logic for detecting the subcommand by finding the first non-flag argument is a bit fragile. While it works for the current CLI structure where all global flags are boolean (like -v), it would fail if a global flag that takes an argument (e.g., --config file.json) were added in the future. In that case, leading would incorrectly capture the flag's argument instead of the subcommand. Consider using a more robust approach or adding a comment to warn future maintainers.
There was a problem hiding this comment.
Good catch, fixed in e926eac. Replaced the argv-scan with parser.parse_known_args() + disabled subparsers declared with add_help=False. Legacy flags (including a hypothetical future value-taking global, or run --config foo.json) now land in unknown and are ignored; the redirect runs based on args.command, which argparse resolves correctly regardless of what globals are defined.
Added test_run_redirect_ignores_unknown_subparser_flags_with_values as a regression guard for the exact class of failure you flagged.
bordeauxred
force-pushed
the
fix/truthful-cli-run
branch
from
6b54c26 to
8e25e15
Compare
bordeauxred
force-pushed
the
fix/truthful-cli-run
branch
from
8e25e15 to
e926eac
Compare
1 participant
Summary
clawloop runandclawloop evalonly wired 2 of 5+ env adapters and failed withUnknown benchmarkon benches the README implies. Per issue Make public CLI and example configs truthful #30, this is a launch-credibility blocker.examples/train_runner.py(the unifiedTrainConfigrunner that covers every supported env) anduv run clawloop demo math --dry-run(the no-key hero demo). Exit code 2.clawloop run --bench entropic ...) gets the redirect instead of an opaque "unrecognized arguments" error.ADAPTER_REGISTRY,_get_adapter,_load_config,_build_evolver,_ensure_output_dir); they had no callers outsidecmd_run/cmd_eval.Post-launch fast-follow: reintroduce
clawloop runas a thin wrapper:clawloop run config.json→train(TrainConfig(**json)). Tracked in a separate issue.Closes #30.
Test plan
pytest tests/test_cli.py— 6 passes:runno-args / with legacy flags / with-v/ with--help,evalno-args,demo math --dry-runstill works.-v run/run --helpregression tests.